-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: [FN-124][FN-125] 알림 생성 및 목록 조회 기능 #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough그룹 가입 요청 시 GroupJoinRequestedEvent를 발행하고 비동기 리스너가 알림을 생성·발송하도록 이벤트 기반 흐름을 추가했다. 알림 엔티티에 groupId/metadata를 도입하고 읽음 처리 API를 전체/단건으로 분리했으며 관련 리포지토리·서비스·문서·메시지와 하이버네이트 배치 설정을 업데이트했다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GroupJoinService
participant EventBus
participant GJRequestedListener
participant NotificationService
participant DB as NotificationRepository
participant GJDispatchListener
participant FCM as PushGateway
Client->>GroupJoinService: 그룹 가입 요청 생성
GroupJoinService->>EventBus: publish GroupJoinRequestedEvent(groupId, receiverIds, requesterId)
EventBus-->>GJRequestedListener: AFTER_COMMIT (async, retryable)
GJRequestedListener->>NotificationService: sendGroupJoinRequest(groupId, receiverIds, requesterId)
NotificationService->>DB: saveAll(notifications)
NotificationService->>EventBus: publish GroupJoinNotificationDispatchEvent(notifications)
EventBus-->>GJDispatchListener: AFTER_COMMIT (async)
GJDispatchListener->>NotificationService: sendGroupJoinRequestNotifications(list)
loop each notification
NotificationService->>FCM: send(message)
end
sequenceDiagram
participant User
participant Controller as NotificationController
participant Service as NotificationService
participant Repo as NotificationRepository
User->>Controller: POST /v1/notifications/read-all
Controller->>Service: markAllNotificationsAsRead(userId)
Service->>Repo: bulkMarkAsRead(userId, now)
Service-->>Controller: 200 OK
User->>Controller: POST /v1/notifications/{id}/read
Controller->>Service: markNotificationAsRead(userId, id)
Service->>Repo: findByIdAndReceiverId(id, userId)
alt not found
Service-->>Controller: NOTIFICATION_NOT_FOUND error
else already read
Service-->>Controller: ALREADY_READ_NOTIFICATION error
else mark read
Service->>Repo: save(entity)
Service-->>Controller: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (21)
src/main/resources/application.yml (1)
19-23: Hibernate 배치 설정 방향성은 좋습니다. MySQL 실제 배치 효율을 위해 드라이버 옵션도 함께 고려해 주세요.
order_inserts,order_updates,hibernate.jdbc.batch_size,hibernate.jdbc.batch_versioned_data설정 조합은 적절합니다.- MySQL에서는 JDBC URL에
rewriteBatchedStatements=true를 포함하지 않으면 실제 네트워크 레벨 배치가 일어나지 않는 경우가 많습니다. 환경변수SPRING_DATASOURCE_URL에 해당 옵션 포함 여부를 확인해 주세요. 예)jdbc:mysql://.../?rewriteBatchedStatements=true- 배치 사이즈(현재 50)는 워크로드에 따라 50~200 사이 튜닝 여지가 큽니다. 모니터링 후 조정 권장.
src/main/java/project/flipnote/common/model/event/GroupJoinRequestedEvent.java (1)
5-10: 이벤트 불변성 강화를 위해 수신자 목록을 방어적 복사로 감싸는 것을 권장합니다.Record라 해도
List<Long>는 가변이므로, 퍼블리셔/리스너 간 사이드 이펙트 방지를 위해 방어적 복사를 적용하면 안전합니다.public record GroupJoinRequestedEvent( Long groupId, List<Long> receiverIds, Long requesterId ) { + public GroupJoinRequestedEvent { + receiverIds = List.copyOf(receiverIds); + } }src/main/java/project/flipnote/group/repository/GroupRolePermissionRepository.java (1)
3-18: 엔티티 대신 롤만 조회하는 경량 메서드 추가 제안현재 메서드는 엔티티 전체를 로딩합니다. 호출부가 결국
GroupMemberRole컬렉션만 필요하다면 롤만 select하는 전용 메서드가 불필요한 연관 로딩을 줄여줍니다.아래처럼 쿼리 메서드를 추가하는 것을 고려해 주세요.
import java.util.List; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; ... boolean existsByGroupAndRoleAndGroupPermission(Group group, GroupMemberRole role, GroupPermission groupPermission); - List<GroupRolePermission> findByGroupAndGroupPermission(Group group, GroupPermission groupPermission); + List<GroupRolePermission> findByGroupAndGroupPermission(Group group, GroupPermission groupPermission); + + @Query("select grp.role from GroupRolePermission grp " + + "where grp.group = :group and grp.groupPermission = :perm") + List<GroupMemberRole> findRolesByGroupAndGroupPermission(@Param("group") Group group, + @Param("perm") GroupPermission groupPermission);
- 장점
- 불필요한
ManyToOne로딩 회피- 호출부에서 곧바로
GroupMemberRepository.findByGroupAndRoleIn(...)에 전달 가능src/main/java/project/flipnote/user/service/UserService.java (1)
127-130: 널 가드 및 활성 사용자 조회 조건 추가 검토
Service 레이어 널 방어
src/main/java/project/flipnote/user/service/UserService.java:127–130public String getNickname(Long userId) { if (userId == null) { throw new BizException(UserErrorCode.USER_NOT_FOUND); } return userProfileRepository.findNicknameById(userId) .orElseThrow(() -> new BizException(UserErrorCode.USER_NOT_FOUND)); }활성 사용자만 조회하도록 리포지토리 쿼리 수정
src/main/java/project/flipnote/user/repository/UserProfileRepository.java:25–27// 기존 @Query("SELECT up.nickname FROM UserProfile up WHERE up.id = :userId") Optional<String> findNicknameById(Long userId); // 제안: up.status = ACTIVE 조건 추가 @Query("SELECT up.nickname FROM UserProfile up " + "WHERE up.id = :userId AND up.status = project.flipnote.user.entity.UserStatus.ACTIVE") Optional<String> findActiveNicknameById(Long userId);또는 메서드 명명 규칙 활용
Optional<String> findNicknameByIdAndStatus(Long userId, UserStatus status);캐싱 고려 (@Cacheable/@CacheEvict)
알림 전파 경로 등 빈번하게 호출되는 경우 유효합니다.src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
25-25: 알림 수신자 수집 시 ID만 조회하는 전용 메서드 추가 제안현재는
GroupMember엔티티 컬렉션을 로딩합니다. 수신자 ID만 필요하다면 사용자/그룹 연관 로딩 오버헤드를 줄이기 위해 ID 전용 조회를 권장합니다.아래 메서드를 추가하면 불필요한 엔티티 로딩을 피할 수 있습니다.
import java.util.List; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; ... List<GroupMember> findByGroupAndRoleIn(Group group, List<GroupMemberRole> roles); + + @Query("select gm.user.id from GroupMember gm " + + "where gm.group = :group and gm.role in :roles") + List<Long> findUserIdsByGroupAndRoleIn(@Param("group") Group group, + @Param("roles") List<GroupMemberRole> roles);
- 장점
- N+1 및 EAGER 연관 로딩 회피
- 대량 알림 전파 경로에서 메모리 사용량 감소
- 참고:
roles가 빈 리스트일 경우 호출부에서 조기 반환하면 불필요한 쿼리를 방지할 수 있습니다.src/main/java/project/flipnote/notification/model/GroupJoinNotificationDispatchEvent.java (2)
7-10: 이벤트 내부 리스트를 불변으로 방어 복사하는 소폭 개선퍼블리시 이후 외부에서 리스트를 변경할 여지를 제거하기 위해 방어 복사를 권장합니다.
아래처럼 compact constructor를 추가하면 됩니다:
public record GroupJoinNotificationDispatchEvent( List<Notification> notifications ) { + public GroupJoinNotificationDispatchEvent { + notifications = java.util.List.copyOf(notifications); + } }
7-10: 이벤트 페이로드에 JPA 엔티티 직접 전달: 현재 위험 없음
현재Notification엔티티는 지연 로딩 대상 연관관계가 없으므로, 비동기(@async) 리스너에서 전달된 엔티티를 그대로 사용해도LazyInitializationException이 발생하지 않습니다.
다만, 이벤트 경계에서 영속성 컨텍스트 의존을 완전히 제거하려면 향후 다음과 같은 Optional 리팩토링을 고려할 수 있습니다:
- 변경 전
public record GroupJoinNotificationDispatchEvent( List<Notification> notifications ) { }- 변경 후 (Optional)
• 호출부에서 ID 리스트로 이벤트 생성public record GroupJoinNotificationDispatchEvent( List<Long> notificationIds ) { }
• 리스너에서notificationRepository.findAllById(notificationIds)등을 통해 엔티티 조회src/main/java/project/flipnote/notification/repository/NotificationRepository.java (2)
24-34: 전체 일괄 읽음 처리 쿼리 변경은 합리적입니다. 파라미터 명을 receiverId로 통일하면 일관성이 좋아집니다.
- findNotificationsByReceiverIdAndCursor에서 receiverId를 사용 중이고, 엔티티 필드도 receiverId입니다. 본 메서드만 userId를 쓰고 있어 혼동 여지 있습니다. JPQL 바인딩 명도 함께 맞추는 것을 권장합니다.
- 성능 관점에서는 (receiver_id, is_read, id DESC) 조합 조회/업데이트가 많다면 DB 인덱스(복합)도 고려해 주세요. 특히 n.read = FALSE 조건과 receiverId 동시 조건이 빈번하면 효과가 큽니다.
적용 예시:
- WHERE n.receiverId = :userId + WHERE n.receiverId = :receiverId AND n.read is FALSE-int bulkMarkAsRead( - @Param("userId") Long userId, - @Param("now") LocalDateTime now -); +int bulkMarkAsRead( + @Param("receiverId") Long receiverId, + @Param("now") LocalDateTime now +);
36-36: 소유권 검증용 조회 메서드 추가 적절합니다. 단건 읽음 처리 시 업데이트 쿼리도 고려해 보세요.
- Optional findByIdAndReceiverId(...)는 올바른 소유권 검증을 보장합니다.
- 추가로, 단건 읽음 처리에서 엔티티 로드 없이 즉시 업데이트하는 리포지토리 메서드도 유용할 수 있습니다(불필요한 SELECT 제거).
예시 제안(새 메서드 추가):
@Modifying(clearAutomatically = true, flushAutomatically = true) @Query(""" UPDATE Notification n SET n.read = TRUE, n.readAt = :now WHERE n.id = :id AND n.receiverId = :receiverId AND n.read = FALSE """) int markAsReadByIdAndReceiverId(@Param("id") Long id, @Param("receiverId") Long receiverId, @Param("now") LocalDateTime now);src/main/java/project/flipnote/notification/entity/Notification.java (2)
36-36: DB 컬럼 명시로 명확성 향상 제안: groupId에 @column(name = "group_id") 추가
- 물리 네이밍 전략이 변경되거나 DDL 자동 생성이 아닌 경우를 대비해 컬럼명을 명시하는 편이 안전합니다.
적용 예시:
- private Long groupId; + @Column(name = "group_id") + private Long groupId;
46-46: metadata 컬럼도 명시하면 이관/마이그레이션 시 혼선을 줄일 수 있습니다.
- 이전 필드(additionalData)에서 metadata로 변경되었습니다. 운영 DB에 컬럼명이 바뀌면 마이그레이션이 필요합니다. 코드에서도 컬럼명을 명시해 두면 의도가 분명합니다.
적용 예시:
- @Convert(converter = MapToJsonConverter.class) - private Map<String, Object> metadata; + @Convert(converter = MapToJsonConverter.class) + @Column(name = "metadata") + private Map<String, Object> metadata;src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java (1)
25-30: Swagger 문서 정밀도 향상: PathVariable 어노테이션을 문서 인터페이스에도 추가 권장
- 구현체에는 @PathVariable이 있지만, 문서 인터페이스에도 추가하면 스키마 추출 시 파라미터 인식이 더 안정적입니다.
적용 예시:
- @Operation(summary = "알림 읽음 처리") - ResponseEntity<Void> markNotificationAsRead(Long notificationId, AuthPrinciple authPrinciple); + @Operation(summary = "알림 읽음 처리") + ResponseEntity<Void> markNotificationAsRead( + @org.springframework.web.bind.annotation.PathVariable("notificationId") Long notificationId, + AuthPrinciple authPrinciple + );또는 파일 상단에 import를 추가해 간결하게 표현할 수 있습니다(선택):
import org.springframework.web.bind.annotation.PathVariable;src/main/java/project/flipnote/notification/listener/GroupJoinNotificationDispatchEventListener.java (1)
20-25: 전송 실패 내성 강화: @Retryable/@recover 추가 또는 재시도 전략 도입 제안
- 현재 서비스 레벨에서 try-catch로 로그만 남깁니다. 일시적 오류(네트워크 등)에 대한 재시도 정책이 없으면 손실될 수 있습니다.
- 다른 유사 리스너(GroupJoinRequestedEventListener, GroupInvitationCreateEventListener)와의 일관성 차이도 있습니다.
적용 예시:
@Async - @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) + @org.springframework.retry.annotation.Retryable( + maxAttempts = 3, + backoff = @org.springframework.retry.annotation.Backoff(delay = 2000, multiplier = 2) + ) + @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void handleGroupJoinNotificationDispatchEvent(GroupJoinNotificationDispatchEvent event) { notificationService.sendGroupJoinRequestNotifications(event.notifications()); } + + @org.springframework.retry.annotation.Recover + public void recover(Exception ex, GroupJoinNotificationDispatchEvent event) { + log.error("그룹 가입 알림 전송 디스패치 실패: notifications={}", event.notifications(), ex); + }주의: 현재 NotificationService.sendGroupJoinRequestNotifications 내부에서 개별 알림 단위로 try-catch 하고 있어, 상단 @retryable이 동작하지 않을 수 있습니다. 재시도 전략을 활용하려면 서비스 레벨에서 예외 전파 방식도 정리해야 합니다.
src/main/java/project/flipnote/notification/controller/NotificationController.java (2)
52-59: API 응답 상태코드 204(No Content) 사용 권장
- 본 작업은 상태 변경성 엔드포인트로 본문이 없어 204가 더 적합합니다. 클라이언트 단 단순성도 증가합니다. (호환성 이슈 없으면 변경 추천)
적용 예시:
@PostMapping("/read-all") public ResponseEntity<Void> markAllNotificationsAsRead( @AuthenticationPrincipal AuthPrinciple authPrinciple ) { notificationService.markAllNotificationsAsRead(authPrinciple.userId()); - return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build(); }
61-68: 단건 읽음 처리 응답 코드 204(No Content)로 통일 제안
- 반환 코드를
ResponseEntity.noContent()(204)로 변경하여 일관성 유지 권장GlobalExceptionHandler의@ExceptionHandler(BizException.class)가
exception.getErrorCode().getStatus()기반으로 404/403 등 적절히 매핑함을 확인했습니다적용 예시:
@PostMapping("/{notificationId}/read") public ResponseEntity<Void> markNotificationAsRead( @PathVariable("notificationId") Long notificationId, @AuthenticationPrincipal AuthPrinciple authPrinciple ) { notificationService.markNotificationAsRead(authPrinciple.userId(), notificationId); - return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build(); }src/main/java/project/flipnote/notification/model/NotificationResponse.java (1)
12-15: API 스키마 변경(groupId/metadata)으로 인한 호환성 영향 점검 필요응답 레코드에 groupId 추가 및 additionalData→metadata로 변경되었습니다. 클라이언트/문서/스냅샷 테스트가 이 변경을 수용하는지 확인하세요. groupId/readAt가 null일 수 있다면 직렬화 정책도 점검이 필요합니다.
권장(선택): null 필드 숨김을 위해 레코드에 JsonInclude 추가를 제안드립니다.
-import com.fasterxml.jackson.annotation.JsonFormat; +import com.fasterxml.jackson.annotation.JsonFormat; +import com.fasterxml.jackson.annotation.JsonInclude; @@ -public record NotificationResponse( +@JsonInclude(JsonInclude.Include.NON_NULL) +public record NotificationResponse(src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (2)
160-167: 권한-역할 조회 최적화(TODO 반영 제안)GroupPermission 엔티티 조회 없이 enum 자체를 키로 사용하는 구조(캐시 또는 자연키)를 도입하면 쿼리를 줄일 수 있습니다. 권한 테이블이 적고 변경이 드문 점을 고려해 간단한 @Cacheable 캐싱도 효과적입니다.
원하시면 Permission 캐시 적용(예: Caffeine/스프링 캐시) 패치를 제안드리겠습니다.
168-173: 중복/자기 자신 제외 처리 제안관리 권한을 가진 신청자가 존재할 수도 있어 자기 자신이 수신자에 포함될 수 있습니다. 중복/본인 제외를 권장합니다.
- List<Long> receiverIds = groupMemberRepository.findByGroupAndRoleIn(group, memberRoles) - .stream() - .map(gm -> gm.getUser().getId()) - .toList(); + List<Long> receiverIds = groupMemberRepository.findByGroupAndRoleIn(group, memberRoles) + .stream() + .map(gm -> gm.getUser().getId()) + .filter(id -> !id.equals(requester.getId())) + .distinct() + .toList();src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (1)
33-39: 재시도-멱등성 관점 점검 권장리스너 재시도 시 NotificationService 레벨에서 멱등성 보장이 없으면 중복 전송 가능성이 있습니다(예: 동일 이벤트 재처리). 자연키 기반 중복 방지나 전송 로그를 고려해보세요.
멱등성 장치가 이미 있다면 공유 부탁드립니다. 필요 시 중복 방지 키 전략 설계를 도와드릴 수 있습니다.
src/main/java/project/flipnote/notification/service/NotificationService.java (2)
178-191: 전송 메서드 트랜잭션 경계 재검토sendGroupJoinRequestNotifications는 @transactional이 없어 내부에서 호출하는 sendNotification의 토큰 삭제/업데이트가 개별 오토커밋으로 실행됩니다. 현재 요구사항에 따라 문제는 없을 수 있으나, 클래스 주석(“반드시 트랜잭션 적용된 public 메서드에서 호출”)과 불일치합니다.
선택지:
- 간단: 주석을 실제 동작에 맞게 정정.
- 일관성: 본 메서드에 @transactional(readOnly = false)을 부여해 토큰 정리 로직을 하나의 트랜잭션으로 묶기.
222-261: 토큰 정리 로직은 합리적이나 buildMessage/variables NPE 가능성 대비 권장Firebase 에러 매핑/불량 토큰 정리는 적절합니다. 다만 variables가 null인 알림 타입이 생길 경우 StringSubstitutor에서 NPE가 날 수 있습니다.
아래처럼 NPE를 방어하면 안전합니다.
- StringSubstitutor substitutor = new StringSubstitutor(notification.getVariables()); + StringSubstitutor substitutor = new StringSubstitutor( + notification.getVariables() != null ? notification.getVariables() : Map.of() + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
src/main/java/project/flipnote/common/model/event/GroupJoinRequestedEvent.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupMemberRepository.java(2 hunks)src/main/java/project/flipnote/group/repository/GroupRolePermissionRepository.java(2 hunks)src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java(6 hunks)src/main/java/project/flipnote/notification/controller/NotificationController.java(2 hunks)src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java(1 hunks)src/main/java/project/flipnote/notification/entity/Notification.java(3 hunks)src/main/java/project/flipnote/notification/entity/NotificationType.java(1 hunks)src/main/java/project/flipnote/notification/exception/NotificationErrorCode.java(1 hunks)src/main/java/project/flipnote/notification/listener/GroupJoinNotificationDispatchEventListener.java(1 hunks)src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java(1 hunks)src/main/java/project/flipnote/notification/model/GroupJoinNotificationDispatchEvent.java(1 hunks)src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java(0 hunks)src/main/java/project/flipnote/notification/model/NotificationResponse.java(2 hunks)src/main/java/project/flipnote/notification/repository/NotificationRepository.java(2 hunks)src/main/java/project/flipnote/notification/service/NotificationService.java(5 hunks)src/main/java/project/flipnote/user/repository/UserProfileRepository.java(2 hunks)src/main/java/project/flipnote/user/service/UserService.java(1 hunks)src/main/resources/application.yml(1 hunks)src/main/resources/messages.properties(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java
🧰 Additional context used
🧬 Code Graph Analysis (17)
src/main/java/project/flipnote/user/repository/UserProfileRepository.java (1)
src/main/java/project/flipnote/auth/repository/UserAuthRepository.java (3)
UserAuthRepository(13-35)Modifying(21-23)Modifying(32-34)
src/main/java/project/flipnote/user/service/UserService.java (1)
src/main/java/project/flipnote/user/model/UserIdNickname.java (3)
UserIdNickname(3-6)getNickname(5-5)getId(4-4)
src/main/java/project/flipnote/common/model/event/GroupJoinRequestedEvent.java (9)
src/main/java/project/flipnote/common/model/event/GroupInvitationCreatedEvent.java (1)
GroupInvitationCreatedEvent(3-7)src/main/java/project/flipnote/groupjoin/model/GroupJoinRequest.java (1)
GroupJoinRequest(3-6)src/main/java/project/flipnote/groupjoin/entity/GroupJoin.java (1)
Getter(22-65)src/main/java/project/flipnote/groupjoin/model/GroupJoinRespondResponse.java (1)
GroupJoinRespondResponse(3-9)src/main/java/project/flipnote/groupjoin/model/GroupJoinResponse.java (1)
GroupJoinResponse(5-11)src/main/java/project/flipnote/groupjoin/model/FindGroupJoinListMeResponse.java (1)
FindGroupJoinListMeResponse(7-13)src/main/java/project/flipnote/groupjoin/model/GroupJoinListResponse.java (1)
GroupJoinListResponse(7-13)src/main/java/project/flipnote/group/model/GroupCreateRequest.java (1)
GroupCreateRequest(6-29)src/main/java/project/flipnote/group/model/GroupDetailResponse.java (1)
GroupDetailResponse(8-41)
src/main/java/project/flipnote/group/repository/GroupRolePermissionRepository.java (3)
src/main/java/project/flipnote/group/entity/GroupRolePermission.java (2)
Entity(11-41)Builder(35-40)src/main/java/project/flipnote/group/entity/GroupPermission.java (1)
Entity(12-29)src/main/java/project/flipnote/group/service/GroupService.java (1)
permission(117-121)
src/main/resources/messages.properties (3)
src/main/java/project/flipnote/groupjoin/model/GroupJoinRequest.java (1)
GroupJoinRequest(3-6)src/main/java/project/flipnote/groupjoin/entity/GroupJoin.java (1)
Getter(22-65)src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (1)
Slf4j(16-37)
src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java (1)
src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java (1)
MarkNotificationsAsReadRequest(7-11)
src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (4)
src/main/java/project/flipnote/notification/listener/GroupJoinNotificationDispatchEventListener.java (1)
Slf4j(13-25)src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (1)
Slf4j(41-284)src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (3)
Slf4j(16-37)Async(23-31)Recover(33-36)src/main/java/project/flipnote/group/listener/UserRegisteredEventListener.java (2)
Slf4j(16-37)Async(23-31)
src/main/java/project/flipnote/notification/exception/NotificationErrorCode.java (6)
src/main/java/project/flipnote/infra/firebase/FcmErrorCode.java (1)
FcmErrorCode(3-21)src/main/java/project/flipnote/common/exception/CommonErrorCode.java (1)
Getter(8-18)src/main/java/project/flipnote/group/exception/GroupInvitationErrorCode.java (1)
Getter(9-25)src/main/java/project/flipnote/common/exception/ErrorCode.java (1)
ErrorCode(3-10)src/main/java/project/flipnote/group/exception/GroupErrorCode.java (1)
Getter(9-26)src/main/java/project/flipnote/auth/exception/AuthErrorCode.java (1)
Getter(9-36)
src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (2)
src/main/java/project/flipnote/group/entity/GroupMember.java (1)
Getter(23-53)src/main/java/project/flipnote/group/entity/GroupRolePermission.java (1)
Entity(11-41)
src/main/java/project/flipnote/notification/entity/NotificationType.java (4)
src/main/java/project/flipnote/groupjoin/entity/GroupJoin.java (1)
Getter(22-65)src/main/java/project/flipnote/groupjoin/model/GroupJoinRequest.java (1)
GroupJoinRequest(3-6)src/main/java/project/flipnote/groupjoin/entity/GroupJoinStatus.java (1)
GroupJoinStatus(3-5)src/main/java/project/flipnote/groupjoin/model/GroupJoinRespondRequest.java (1)
GroupJoinRespondRequest(5-8)
src/main/java/project/flipnote/notification/model/GroupJoinNotificationDispatchEvent.java (2)
src/main/java/project/flipnote/groupjoin/entity/GroupJoin.java (1)
Getter(22-65)src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (1)
Slf4j(16-37)
src/main/java/project/flipnote/notification/entity/Notification.java (1)
src/main/java/project/flipnote/common/model/event/GroupInvitationCreatedEvent.java (1)
GroupInvitationCreatedEvent(3-7)
src/main/java/project/flipnote/notification/controller/NotificationController.java (1)
src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java (1)
MarkNotificationsAsReadRequest(7-11)
src/main/java/project/flipnote/notification/listener/GroupJoinNotificationDispatchEventListener.java (3)
src/main/java/project/flipnote/notification/service/NotificationService.java (1)
Slf4j(42-294)src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (1)
Slf4j(16-40)src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (3)
Slf4j(16-37)Async(23-31)Recover(33-36)
src/main/java/project/flipnote/notification/repository/NotificationRepository.java (2)
src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java (1)
MarkNotificationsAsReadRequest(7-11)src/main/java/project/flipnote/notification/repository/FcmTokenRepository.java (2)
FcmTokenRepository(14-25)Modifying(20-22)
src/main/java/project/flipnote/notification/service/NotificationService.java (2)
src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (2)
Slf4j(16-37)Async(23-31)src/main/java/project/flipnote/group/service/GroupInvitationService.java (1)
RequiredArgsConstructor(34-285)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (3)
src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (1)
Slf4j(16-40)src/main/java/project/flipnote/groupjoin/entity/GroupJoin.java (1)
Getter(22-65)src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java (1)
Repository(12-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
src/main/java/project/flipnote/notification/entity/NotificationType.java (1)
9-10: 알림 타입 및 엔티티 매핑 재확인 필요
GROUP_JOIN_REQUEST("notification.group.join.request")메시지 키가messages.properties(및 각 언어별 프로퍼티) 파일에 등록되어 있는지, 번역까지 포함해 확인해 주세요.Notification엔티티의NotificationType필드에@Enumerated(EnumType.STRING)애노테이션이 적용되어 있는지 스크립트로 확인했으나 출력 결과가 없었습니다. 데이터 호환성을 위해 코드에서 해당 애노테이션 사용 여부를 직접 검토해 주세요.src/main/java/project/flipnote/notification/exception/NotificationErrorCode.java (1)
13-15: 새 에러 코드 추가 LGTM.
NOTIFICATION_NOT_FOUND,ALREADY_READ_NOTIFICATION추가로 읽기 API의 예외 표현력이 좋아졌습니다. 서비스 계층에서 해당 코드로 매핑되는지 확인만 부탁드립니다.src/main/resources/messages.properties (1)
2-2: 플레이스홀더 바인딩 정상 확인 완료NotificationService.sendGroupJoinRequest에서
.variables(Map.of("requesterNickname", requesterNickname))로 정상 바인딩되고 있어,
${requesterNickname}가 그대로 노출될 우려가 없습니다.
별도 수정 없이 머지 진행해 주세요.src/main/java/project/flipnote/group/repository/GroupRolePermissionRepository.java (1)
17-17: 역할 해석용 조회 메서드 추가, 용도에 적합합니다
findByGroupAndGroupPermission으로 특정 권한에 매핑된 역할들을 조회하는 흐름이 명확합니다. 그룹 가입 알림 수신자 롤 해석에 적절해 보입니다.src/main/java/project/flipnote/notification/repository/NotificationRepository.java (1)
5-5: Optional import 추가 적절합니다.
- 새 메서드 반환 타입과 일치합니다.
src/main/java/project/flipnote/notification/listener/GroupJoinNotificationDispatchEventListener.java (1)
20-24: receiverIds 추출 로직에서 GroupMember PK(id) 대신 실제 유저 ID 사용 필요GroupJoinService에서 수신자 ID를
.map(GroupMember::getId)로 수집하고 있어, 이는 GroupMember 엔티티의 기본키를 가져옵니다. 알림 수신 대상이 유저 ID라면gm.getUser().getId()또는gm.getUserId()로 변경해야 합니다.수정 대상:
- 파일:
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java- 위치: 약 168~172라인
제안하는 코드 변경 예시:
- List<Long> receiverIds = groupMemberRepository.findByGroupAndRoleIn(group, memberRoles) - .stream() - .map(GroupMember::getId) - .toList(); + List<Long> receiverIds = groupMemberRepository.findByGroupAndRoleIn(group, memberRoles) + .stream() + .map(gm -> gm.getUser().getId()) // 또는 .map(GroupMember::getUserId) + .toList();
GroupMember엔티티에getUserId()가 제공되지 않는다면gm.getUser().getId()를 사용하세요.- 변경 후에도
sendGroupJoinRequest호출부와 이벤트 전파 로직이 올바르게 동작하는지 검증 부탁드립니다.Likely an incorrect or invalid review comment.
src/main/java/project/flipnote/notification/model/NotificationResponse.java (1)
24-33: 정적 팩토리 메서드 업데이트 적절Notification에서 groupId/metadata를 안전하게 위임하고, 메시지를 외부에서 생성해 주입하는 방향은 응답 모델의 책임 분리에 부합합니다.
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (1)
75-83: 최대 인원 체크에 행 잠금 사용 적절FOR UPDATE로 그룹 행을 잠그고 카운팅하는 방식은 동시성 경쟁을 완화합니다. 응답 시에도 동일 루틴을 재사용하여 일관성을 유지하고 있습니다.
src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (1)
23-31: 비동기+재시도+커밋 후 처리 구성 적절AFTER_COMMIT에서 비동기로 NotificationService를 호출하는 패턴이 도메인 트랜잭션과 알림 전송을 잘 분리합니다. 재시도 설정도 합리적입니다.
src/main/java/project/flipnote/notification/service/NotificationService.java (1)
154-170: 템플릿 키/변수 정의 확인 완료
src/main/resources/messages.properties에notification.group.join.request키가 존재하고,${requesterNickname}변수를 사용하고 있음NotificationType.GROUP_JOIN_REQUEST("notification.group.join.request")가NotificationTypeenum에 올바르게 정의되어 있음모든 검증이 완료되었습니다. 코드 변경을 승인합니다.
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java
Outdated
Show resolved
Hide resolved
src/main/java/project/flipnote/user/repository/UserProfileRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/java/project/flipnote/notification/service/NotificationService.java (1)
119-134: FCM 토큰 재할당 누락: 다른 사용자 소유 토큰 삭제 후 재저장이 필요합니다기존 토큰이 다른 사용자에게 매핑되어 있으면 삭제만 하고 현재 사용자에 대한 재저장이 이뤄지지 않습니다. 결과적으로 토큰이 미등록 상태가 됩니다.
아래 패치로 삭제 후 재등록을 보완해 주세요.
public void registerFcmToken(Long userId, TokenRegisterRequest req) { Optional<FcmToken> existingToken = fcmTokenRepository.findByToken(req.token()); if (existingToken.isPresent()) { FcmToken token = existingToken.get(); if (Objects.equals(token.getUserId(), userId)) { token.updateLastUsedAt(); } else { fcmTokenRepository.deleteById(token.getId()); + // 동일 토큰을 현재 사용자에게 재할당 + saveFcmToken(userId, req.token()); } } else { saveFcmToken(userId, req.token()); } }
🧹 Nitpick comments (4)
src/main/java/project/flipnote/common/model/request/CursorPageRequest.java (1)
21-37: 비정상 커서 값을 무음(null) 처리할지, 400(Bad Request)로 명확히 실패시킬지 정책 합의가 필요합니다현재는 "abc", "1e10", "99999999999999999999" 같은 입력을 모두 null로 간주하여 첫 페이지를 반환합니다. 클라이언트 디버깅/오류 탐지 측면에선 명시적 검증이 더 유리합니다.
가능한 개선안:
- 검증 어노테이션으로 유효성 선제 차단: cursor에
@Pattern(regexp = "\\s*\\d+\\s*")적용해 숫자만 허용- 음수/0 커서는 무효 처리: parse 이후
<= 0이면 null 반환- API 정책상 잘못된 커서는 400으로 응답하도록 컨트롤러 레이어에서 바인딩 에러 처리
원하시면 위 정책에 맞춘 변경 패치를 제안드리겠습니다.
src/main/java/project/flipnote/notification/model/NotificationListRequest.java (1)
8-16: 문서화 보강: 스웨거 스키마/예시 추가를 권장합니다API 소비자 경험을 위해 필터 의미와 예시 값을 명시하면 좋습니다.
예시:
- groupId: 특정 그룹의 알림만 조회 (예: 123)
- read: 읽음 여부 필터 (예: true면 읽은 알림만)
원하시면@Schema(description = "...", example = "...")추가 패치 드리겠습니다.src/main/java/project/flipnote/notification/repository/NotificationRepository.java (1)
17-30: 대량 조회 성능 최적화 제안: 인덱스 확인조건절이
receiverId(ANDid < cursor) 중심이므로 아래 인덱스 고려를 권합니다.
- 복합 인덱스: (receiver_id, id DESC) — 커서 조건과 정렬 동시 활용
- 추가 필터 사용 빈도에 따라: (receiver_id, group_id, is_read, id DESC)
인덱스 전략은 실제 질의 빈도/카디널리티 기반으로 최적화가 필요합니다.src/main/java/project/flipnote/notification/service/NotificationService.java (1)
100-110: 그룹 초대 알림 전송의 트랜잭션 롤백 전파 주의
sendNotification실패 시 BizException을 던져 알림 저장까지 롤백될 수 있습니다. 그룹 가입 신청 쪽은 이벤트 기반 비동기로 전송을 분리했는데, 초대 역시 동일한 패턴으로 일관화하는 것을 권장합니다.대안:
- 알림 엔티티 저장은 커밋 보장, 전송은 이벤트 리스너(@Async/@TransactionalEventListener)로 위임
- 혹은 전송 실패를 로깅 후 삼키고 재처리 큐 도입
원하시면 초대 전송 흐름도 이벤트 기반으로 리팩터 제안 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/project/flipnote/common/model/request/CursorPageRequest.java(1 hunks)src/main/java/project/flipnote/notification/model/NotificationListRequest.java(1 hunks)src/main/java/project/flipnote/notification/repository/NotificationRepository.java(3 hunks)src/main/java/project/flipnote/notification/service/NotificationService.java(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/project/flipnote/notification/model/NotificationListRequest.java (3)
src/main/java/project/flipnote/common/model/request/CursorPageRequest.java (1)
Getter(11-38)src/main/java/project/flipnote/notification/controller/NotificationController.java (1)
RequiredArgsConstructor(24-61)src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java (1)
Tag(14-28)
src/main/java/project/flipnote/notification/service/NotificationService.java (5)
src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (2)
Slf4j(16-37)Async(23-31)src/main/java/project/flipnote/notification/controller/NotificationController.java (1)
RequiredArgsConstructor(24-61)src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java (1)
Tag(14-28)src/main/java/project/flipnote/notification/entity/Notification.java (1)
Getter(23-68)src/main/java/project/flipnote/group/service/GroupInvitationService.java (1)
RequiredArgsConstructor(34-285)
src/main/java/project/flipnote/notification/repository/NotificationRepository.java (3)
src/main/java/project/flipnote/notification/repository/FcmTokenRepository.java (1)
FcmTokenRepository(14-25)src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java (1)
MarkNotificationsAsReadRequest(7-11)src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java (1)
Tag(14-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
src/main/java/project/flipnote/common/model/request/CursorPageRequest.java (1)
32-36: 숫자 파싱 실패에 대한 안전한 처리 추가는 적절합니다비정상 커서 입력에 대해 NumberFormatException을 삼키고 null을 반환함으로써 조회 API가 안전하게 동작합니다. (레거시 클라이언트/오입력 호환성 측면에서 유리)
src/main/java/project/flipnote/notification/model/NotificationListRequest.java (1)
12-16: 그룹/읽음 상태 필터 추가 방향성 좋습니다
- groupId에 @min(1)으로 음수 방지한 점 적절합니다.
- read를 Boolean으로 선언해 필터 미적용(null)/true/false 3가지를 구분하는 접근도 맞습니다.
src/main/java/project/flipnote/notification/repository/NotificationRepository.java (4)
17-30: 커서/필터 조합 쿼리 구성 적절합니다
- desc 정렬(서비스 단 Sort) +
id < :cursor조건이 커서 페이징 규칙과 일관됩니다.- groupId/read를 선택적 필터로 둔 점도 유연합니다.
44-44: 단건 조회 보호 조건 추가 훌륭합니다
findByIdAndReceiverId도입으로 사용자 소유 알림만 접근 가능하도록 한 점 좋습니다.
17-23: JDK 15+ 필요: Java 텍스트 블록(""") 사용
"""텍스트 블록은 JDK 15 이상(권장 JDK 17)에서만 지원됩니다. 빌드 설정에서 아래 항목을 확인해 주세요:
- Gradle
build.gradle/build.gradle.kts에 설정된sourceCompatibility/targetCompatibility- 또는 Java Toolchain 설정 (
java { toolchain { languageVersion.set(JavaLanguageVersion.of(17)) } })- Maven
pom.xml의maven-compiler-pluginmaven.compiler.source/maven.compiler.target위 설정이 JDK 15 이상으로 맞춰져 있는지 검증 후 반영 부탁드립니다.
32-42: 전체 읽음 일괄 업데이트 구현 및 ID 기반 로직 제거 확인다음과 같이 검증되었습니다.
- NotificationRepository.bulkMarkAsRead 쿼리(
@Modifying … UPDATE …)가 정상 적용됨- NotificationService.markAllNotificationsAsRead → bulkMarkAsRead 호출
- NotificationController에
/read-all엔드포인트로 전체 읽음 처리 구현- 기존 ID 목록 기반 호출(markNotificationsAsRead) 및
MarkNotificationsAsReadRequestDTO 모두 제거- 단일 알림 읽음 처리(
/\{notificationId\}/read)는 별도 유지추가 조치 없이 이대로 머지 가능합니다.
src/main/java/project/flipnote/notification/service/NotificationService.java (4)
66-69: 커서 페이징 처리 로직 일관성 좋습니다
- size+1 조회로 hasNext 판정 후 마지막 항목 id를 nextCursor로 전달하는 방식 적절합니다.
202-212: 단건 읽음 처리 로직 명확하고 안전합니다
- 소유자 검증 후 읽음 상태 중복 처리 방지까지 적절합니다.
156-172: 빌더 필드 정합성 검증 완료
Notification.java에서 아래 사항을 확인했습니다:
private Long groupId;private Map<String, Object> metadata;@Builder생성자에groupId및metadata파라미터가 모두 정의됨따라서
sendGroupJoinRequest메서드에서 사용하는 빌더 호출에는 문제가 없습니다.
137-146: 전체 읽음 API 컨트롤러/문서 정합성 확인 완료
NotificationController.java에서POST /read-all➔markAllNotificationsAsRead(AuthPrinciple)호출이 정상 반영되어 있습니다.- Swagger 인터페이스(
NotificationControllerDocs.java)에도markAllNotificationsAsRead(AuthPrinciple authPrinciple)및 “모든 알림 읽음 처리” 요약이 일치합니다.- 이전의
MarkNotificationsAsReadRequestDTO 및markNotificationsAsRead(...)호출은 모두 제거되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (2)
170-176: 알림 수신자 ID 매핑 오류: 그룹 ID를 수신자 ID로 사용 중알림 수신자 ID를
groupMember.getGroup().getId()로 매핑하고 있어, 실제 유저 ID가 아닌 그룹 ID가 전달됩니다. NotificationService는 수신자(userId) 기준으로 동작하므로 전송 실패/오발송의 원인이 됩니다. 유저 ID로 매핑하세요.적용 diff:
- List<Long> receiverIds = groupMemberRepository.findByGroupAndRoleIn(group, memberRoles) - .stream() - .map((groupMember -> groupMember.getGroup().getId())) - .toList(); + List<Long> receiverIds = groupMemberRepository.findByGroupAndRoleIn(group, memberRoles) + .stream() + .map(gm -> gm.getUser().getId()) + .toList();
147-151: 자동 가입(applicationRequired=false) 시 그룹 멤버 추가 누락
applicationRequired=false인 경우status=ACCEPT로 저장되지만 실제GroupMember추가가 없습니다. 자동 가입 시에는 즉시 멤버를 추가해야 일관성이 맞습니다. (이전 리뷰 코멘트와 동일 이슈)적용 diff:
- if (group.getApplicationRequired()) { - sendJoinRequestNotification(group, user); - } + if (group.getApplicationRequired()) { + sendJoinRequestNotification(group, user); + } else { + // 자동 가입: 그룹 멤버 즉시 추가 + GroupMember groupMember = GroupMember.builder() + .group(group) + .user(user) + .role(GroupMemberRole.MEMBER) + .build(); + groupMemberRepository.save(groupMember); + }
🧹 Nitpick comments (3)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (3)
129-131: 최대 인원 선검사 위치 개선 제안대기(PENDING) 플로우에서도 선제적으로 인원 제한을 검사하면, 현재 만석일 때 신청 자체가 불가능합니다. 일반적으로 인원 검사는 승인 시점에만 필요합니다. 자동 가입일 때만 선 검사로 제한하는 것을 권장합니다.
적용 diff:
- //그룹이 최대인원인 경우 - checkMaxMember(group); + // 그룹이 최대인원인 경우(자동 가입 시에만 선 체크) + if (!group.getApplicationRequired()) { + checkMaxMember(group); + }
230-238: 중복 멤버 추가 방지(멱등성)승인 처리 시 이미 가입된 유저를 다시 추가하면 데이터 무결성 오류(중복 키) 가능성이 있습니다. 저장 전에 존재 여부를 확인해 멱등하게 처리하는 것을 권장합니다.
적용 diff:
- groupMemberRepository.save(groupMember); + if (groupMemberRepository.findByGroupAndUser(group, groupJoin.getUser()).isEmpty()) { + groupMemberRepository.save(groupMember); + }추가로, 자동 가입 경로에서도 동일한 체크를 적용하면 더 안전합니다.
68-71: 중복 가입신청 판정 로직 개선 제안
existsByGroup_idAndUser_id는 신청 상태와 무관하게 존재만 보면 true를 반환합니다. REJECT/CANCEL 이후 재신청을 허용하려면 상태 조건(PENDING/ACCEPT 등)에 한정하거나, 비즈니스 요구에 맞는 상태 기반 exists 쿼리를 도입하는 것이 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java(6 hunks)src/main/java/project/flipnote/user/repository/UserProfileRepository.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/project/flipnote/user/repository/UserProfileRepository.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (5)
src/main/java/project/flipnote/notification/service/NotificationService.java (1)
Slf4j(43-296)src/main/java/project/flipnote/notification/listener/GroupJoinNotificationDispatchEventListener.java (1)
Slf4j(13-25)src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (1)
Slf4j(16-40)src/main/java/project/flipnote/groupjoin/entity/GroupJoin.java (1)
Getter(22-65)src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java (1)
Repository(12-19)
🔇 Additional comments (1)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (1)
73-83: 동시성 안전한 최대 인원 체크 도입 좋습니다
findByIdForUpdate로 그룹 행을 잠근 뒤 카운트 비교하는 방식은 경합 상황에서도 용량 초과를 방지합니다. 설계 적합합니다.
| //그룹 가입 신청 조회 | ||
| GroupJoin groupJoin = findGroupJoin(joinId); | ||
|
|
||
| //최대 인원 조회 | ||
| if (req.status() == GroupJoinStatus.ACCEPT) { | ||
| checkMaxMember(group); | ||
| // 업데이트 | ||
| groupJoin.updateStatus(req.status()); | ||
|
|
||
| groupJoinRepository.save(groupJoin); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
respondToJoinRequest: joinId와 groupId 정합성 검증 누락
joinId로 조회한 GroupJoin이 현재 groupId에 속하는지 검증이 없습니다. 다른 그룹의 신청을 승인/거절할 수 있는 취약점입니다. 삭제 API에서는 검증을 하고 있으니 동일하게 맞춰주세요.
적용 diff:
//그룹 가입 신청 조회
GroupJoin groupJoin = findGroupJoin(joinId);
+ // joinId가 현재 groupId에 속하는지 검증
+ if (!groupJoin.getGroup().getId().equals(groupId)) {
+ throw new BizException(GroupJoinErrorCode.USER_NOT_PERMISSION);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //그룹 가입 신청 조회 | |
| GroupJoin groupJoin = findGroupJoin(joinId); | |
| //최대 인원 조회 | |
| if (req.status() == GroupJoinStatus.ACCEPT) { | |
| checkMaxMember(group); | |
| // 업데이트 | |
| groupJoin.updateStatus(req.status()); | |
| groupJoinRepository.save(groupJoin); | |
| //그룹 가입 신청 조회 | |
| GroupJoin groupJoin = findGroupJoin(joinId); | |
| // joinId가 현재 groupId에 속하는지 검증 | |
| if (!groupJoin.getGroup().getId().equals(groupId)) { | |
| throw new BizException(GroupJoinErrorCode.USER_NOT_PERMISSION); | |
| } | |
| //최대 인원 조회 | |
| if (req.status() == GroupJoinStatus.ACCEPT) { | |
| checkMaxMember(group); | |
| // 업데이트 | |
| groupJoin.updateStatus(req.status()); | |
| groupJoinRepository.save(groupJoin); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java around
lines 219 to 229, the code fetches a GroupJoin by joinId but does not verify it
belongs to the current groupId, allowing cross-group approval/rejection; add a
guard after findGroupJoin(joinId) that compares the GroupJoin's group id with
the provided group's id (or groupId) and throw an appropriate exception (same
type/message used in the delete API) if they differ, then proceed with
checkMaxMember, updateStatus and save.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P5: 이 부분 제가 수정하는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stoneTiger0912 네, 맞습니다! 이 부분을 수정해주시면 좋겠습니다.
비록 P5 우선순위이지만 보안과 관련된 이슈이므로 수정하는 것이 좋습니다. 제가 제안한 코드를 참고해서 groupJoin.getGroup().getId().equals(groupId) 검증 로직을 추가해주시면 됩니다. 삭제 API에서 이미 동일한 패턴으로 검증하고 있으니 일관성도 맞출 수 있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stoneTiger0912 옙 수정하면 좋을 것 같습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/java/project/flipnote/notification/service/NotificationService.java (1)
121-134: 다른 사용자 소유의 FCM 토큰 재등록 시 저장 누락 버그기존 토큰이 타 사용자 소유인 경우, 삭제만 수행하고 현재 사용자로 재등록을 하지 않아 최종적으로 토큰이 사라집니다. 삭제 후 즉시 현재 사용자로 저장이 필요합니다.
다음 수정안을 적용해 주세요:
if (existingToken.isPresent()) { FcmToken token = existingToken.get(); if (Objects.equals(token.getUserId(), userId)) { token.updateLastUsedAt(); } else { - fcmTokenRepository.deleteById(token.getId()); + fcmTokenRepository.deleteById(token.getId()); + // 동일 물리 토큰을 현재 사용자로 재등록 + saveFcmToken(userId, req.token()); } } else { saveFcmToken(userId, req.token()); }
🧹 Nitpick comments (3)
src/main/java/project/flipnote/notification/service/NotificationService.java (3)
156-172: 수신자 리스트 전처리(중복/자기 자신 제외/널 가드) 권장실수로 동일 ID가 중복되거나 신청자가 자기 자신을 포함시키는 경우를 방지하세요. 또한 널 안전성 확보를 권장합니다.
아래처럼 스트림 전처리를 추가해 주세요:
- List<Notification> notifications = receiverIds.stream() - .map((receiverId) -> Notification.builder() + List<Notification> notifications = receiverIds.stream() + .filter(Objects::nonNull) + .filter(receiverId -> !Objects.equals(receiverId, requesterId)) + .distinct() + .map(receiverId -> Notification.builder() .receiverId(receiverId) .groupId(groupId) .type(type) .variables(Map.of("requesterNickname", requesterNickname)) .metadata(Map.of("requesterId", requesterId)) .build()) .toList();
180-193: 전송 실패 처리 전략 고도화 제안현재는 try/catch + 로그만 존재합니다. 운영 신뢰성을 위해 다음을 고려해 주세요.
- 이벤트 리스너(@async + @retryable)로 재시도(지수 백오프) 처리
- 실패 건을 DLQ/아웃박스 테이블에 적재 후 배치 재전송
- 사용자별 토큰 갱신/삭제는 이미 적용되어 좋습니다
원하시면 간단한 아웃박스 엔티티/스케줄러 설계를 제안드리겠습니다.
238-243: 개별 응답의 예외 null 가드 추가 제안Firebase SDK에서 실패 응답은 보통 예외가 있으나, 방어적으로 null 가드를 추가해 NPE를 예방하는 것이 안전합니다.
- String errorName = res.getException().getMessagingErrorCode().name(); + String errorName = (res.getException() != null && res.getException().getMessagingErrorCode() != null) + ? res.getException().getMessagingErrorCode().name() + : "INTERNAL";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
src/main/java/project/flipnote/auth/controller/AuthController.java(1 hunks)src/main/java/project/flipnote/auth/controller/docs/AuthControllerDocs.java(1 hunks)src/main/java/project/flipnote/auth/service/AuthService.java(2 hunks)src/main/java/project/flipnote/common/exception/CommonErrorCode.java(1 hunks)src/main/java/project/flipnote/common/security/config/SecurityConfig.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupInvitationRepository.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupMemberRepository.java(1 hunks)src/main/java/project/flipnote/group/service/GroupService.java(3 hunks)src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java(1 hunks)src/main/java/project/flipnote/infra/firebase/FirebaseService.java(1 hunks)src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java(1 hunks)src/main/java/project/flipnote/notification/repository/NotificationRepository.java(2 hunks)src/main/java/project/flipnote/notification/service/NotificationService.java(7 hunks)src/main/java/project/flipnote/user/model/UserIdNickname.java(1 hunks)src/main/java/project/flipnote/user/repository/UserProfileRepository.java(2 hunks)src/main/java/project/flipnote/user/service/UserService.java(2 hunks)
✅ Files skipped from review due to trivial changes (9)
- src/main/java/project/flipnote/user/model/UserIdNickname.java
- src/main/java/project/flipnote/auth/controller/docs/AuthControllerDocs.java
- src/main/java/project/flipnote/group/repository/GroupInvitationRepository.java
- src/main/java/project/flipnote/common/security/config/SecurityConfig.java
- src/main/java/project/flipnote/auth/controller/AuthController.java
- src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java
- src/main/java/project/flipnote/infra/firebase/FirebaseService.java
- src/main/java/project/flipnote/common/exception/CommonErrorCode.java
- src/main/java/project/flipnote/auth/service/AuthService.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java
- src/main/java/project/flipnote/user/repository/UserProfileRepository.java
- src/main/java/project/flipnote/user/service/UserService.java
- src/main/java/project/flipnote/group/repository/GroupMemberRepository.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/project/flipnote/notification/service/NotificationService.java (4)
src/main/java/project/flipnote/notification/listener/GroupInvitationCreateEventListener.java (2)
Slf4j(16-37)Async(23-31)src/main/java/project/flipnote/notification/controller/NotificationController.java (1)
RequiredArgsConstructor(24-61)src/main/java/project/flipnote/notification/controller/docs/NotificationControllerDocs.java (1)
Tag(14-28)src/main/java/project/flipnote/notification/entity/Notification.java (1)
Getter(23-68)
src/main/java/project/flipnote/group/service/GroupService.java (3)
src/test/java/project/flipnote/group/service/GroupServiceTest.java (1)
ExtendWith(43-208)src/main/java/project/flipnote/group/entity/Group.java (1)
Getter(28-101)src/main/java/project/flipnote/group/entity/GroupPermission.java (1)
Entity(12-29)
src/main/java/project/flipnote/notification/repository/NotificationRepository.java (3)
src/main/java/project/flipnote/notification/repository/FcmTokenRepository.java (1)
FcmTokenRepository(14-25)src/main/java/project/flipnote/notification/entity/Notification.java (1)
Getter(23-68)src/main/java/project/flipnote/notification/model/MarkNotificationsAsReadRequest.java (1)
MarkNotificationsAsReadRequest(7-11)
🔇 Additional comments (12)
src/main/java/project/flipnote/group/service/GroupService.java (5)
152-155: 오너 멤버 초기 저장 로직은 적절합니다Builder 사용, 트랜잭션 경계 내 저장 흐름 모두 자연스럽습니다. (DB 레벨에서 그룹당 OWNER 1명 제약이 있다면 중복 생성 방지도 확실해집니다.)
53-53: 포맷팅 변경만 존재논리 변경 없음. 별도 조치 불필요합니다.
57-60: existsByGroup_idAndUser_id 시그니처 일치 확인
- src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (line 20):
boolean existsByGroup_idAndUser_id(Long groupId, Long userId);- src/main/java/project/flipnote/group/service/GroupService.java (line 57):
return groupMemberRepository.existsByGroup_idAndUser_id(groupId, user.getId());서비스 호출과 레포지토리 선언이 완전히 일치하므로, 런타임 파싱 오류 없이 정상 동작합니다. 제안된 메서드명 변경은 필요 없습니다.
Likely an incorrect or invalid review comment.
114-121: 권한 초기화 및 JDK 버전 검증 요청
- 파일:
src/main/java/project/flipnote/group/service/GroupService.java(114–121)현재 아래 로직은 모든
GroupMemberRole× 모든groupPermissions조합을 생성합니다. 이대로라면MEMBER/VIEWER같은 일반 역할에도KICK/INVITE등 관리자 권한이 부여될 수 있어, 권한 상승 위험이 큽니다. 의도된 정책인지 확인 부탁드립니다.만약 OWNER 에만 전체 권한을 부여하고 나머지 역할은 최소 권한으로 시작하려면 아래와 같이 변경을 고려해 보세요:
- List<GroupRolePermission> groupRolePermissions = Arrays.stream(GroupMemberRole.values()) - .flatMap(role -> groupPermissions.stream() - .map(permission -> GroupRolePermission.builder() - .group(group) - .groupPermission(permission) - .role(role) - .build())) - .toList(); + List<GroupRolePermission> groupRolePermissions = groupPermissions.stream() + .map(permission -> GroupRolePermission.builder() + .group(group) + .groupPermission(permission) + .role(GroupMemberRole.OWNER) + .build()) + .toList();추가로,
Stream.toList()는 Java 16 이상에서만 제공되는 메서드입니다.
프로젝트의 타깃 JDK 버전이 16+ 인지 확인하시고, 그렇지 않다면
collect(Collectors.toList())사용- 또는 빌드 설정(
sourceCompatibility/targetCompatibility또는maven.compiler.source/target)을 Java 16 이상으로 업데이트위 두 가지 사항을 검토해 주세요.
129-138: 불필요한 수정 제안:createGroup메서드에서req.image()는 DTO에 선언된String image필드를 올바르게 참조합니다. 제안된req.imageUrl()접근자는 존재하지 않으므로, 현행 코드를 유지해 주세요.Likely an incorrect or invalid review comment.
src/main/java/project/flipnote/notification/repository/NotificationRepository.java (3)
32-42: 여러 알림 → 전체 읽음 처리로 동작 변경됨. 상위 API/DTO와의 계약 충돌 가능성 확인 필요이제 특정 ID 목록이 아닌, 사용자 기준 미읽음 전체를 일괄 갱신합니다. 현재 코드베이스에
MarkNotificationsAsReadRequest(List<Long> notificationIds)가 남아 있고 컨트롤러도 해당 DTO를 받는다면, 서비스/컨트롤러 간 계약 불일치로 컴파일/런타임 이슈가 날 수 있습니다. 상위 레이어(Controller/Docs/Client) 동기화를 확인/정리해 주세요.위 리뷰의 검증 스크립트 [2][3] 섹션으로 빠르게 확인 가능합니다. 필요시 컨트롤러를 전체 읽음 처리 엔드포인트와 단건 읽음 처리(PATCH /{id}/read 등)로 분리하는 것을 제안합니다.
44-44: 리시버 소유권을 포함한 조회 메서드 추가 적절합니다단건 읽음 처리 시 NotFound/AlreadyRead 판단을 안전하게 할 수 있어 도메인 규칙에 잘 맞습니다.
17-30: 커서 기반 페이징/동적 필터: 매핑 검증 완료 & 인덱스 권장
Notification엔티티에groupId(Long),metadata(Map<String,Object>),boolean read필드(@Column(name="is_read"))가 모두 정의·매핑되어 있어 JPQL의n.groupId및n.read참조가 유효합니다.- 컨트롤러(
NotificationController)와 서비스(NotificationService) 간markNotificationAsRead,markAllNotificationsAsRead메서드 시그니처가 일치함을 확인했습니다.- 대용량 처리 성능을 위해
notifications테이블에(receiver_id, is_read, group_id, id DESC)순의 복합 인덱스(또는 적절한 단일/복합 인덱스) 도입을 검토하세요.src/main/java/project/flipnote/notification/service/NotificationService.java (4)
66-69: 커서 페이징 구성 적절합니다
- size+1 조회 → hasNext 판정 → 마지막 id를 nextCursor로 반환하는 패턴이 올바릅니다.
- 정렬(id desc)과 리포지토리 커서 조건(id < :cursor)이 일치합니다.
100-106: 그룹 초대 알림에 groupId를 직접 필드로 저장하는 방향 OK템플릿 변수는
groupName만 사용하고, 그룹 식별은 엔티티 필드groupId로 분리하여 조회/필터링이 수월해집니다.
137-145: 전체 읽음 처리로 API가 바뀐 만큼 컨트롤러/문서/클라이언트 동기화 필요컨트롤러가 여전히
MarkNotificationsAsReadRequest(ID 리스트)를 받고 있다면 계약 충돌입니다. 엔드포인트를 다음처럼 분리하는 것을 권장합니다:
- POST /v1/notifications/read/all → 전체 읽음 처리
- PATCH /v1/notifications/{notificationId}/read → 단건 읽음 처리
앞선 리포지토리 코멘트에 포함된 스크립트 [2][3]로 실제 참조 상태를 확인해 주세요. 필요시 컨트롤러/Docs 업데이트 PR 분리를 도와드릴 수 있습니다.
202-212: 단건 읽음 처리: 소유권 검증/중복 처리 로직 적절
- 다른 사용자의 알림 접근 차단
- 이미 읽음이면 도메인 예외 반환
- 도메인 메서드로 상태 전이 캡슐화
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
신규 기능
변경 사항
오류 처리
작업/성능